Summary By MatterAI

🔄 What Changed

This Pull Request introduces a crucial null check within the genericHTTPRequest method in BNCServerInterface.m. Previously, the code attempted to access serverResponse.data[@"error"] directly, which could lead to a crash if serverResponse.data itself was nil. The change adds a defensive check if(serverResponse.data && serverResponse.data[@"error"] != nil) to ensure serverResponse.data is not nil before attempting to access its keys. Additionally, a redundant NSLog statement and an inner BranchLogger call were removed, streamlining the error logging process.

🔍 Impact of the Change

This change significantly improves the robustness and stability of the SDK by preventing potential crashes when a network request is successful but returns a nil or unexpected data payload from the service. It ensures that an NSError object is only created and populated when valid error data is present, leading to more reliable error handling and logging. The removal of redundant logging also cleans up the codebase.

📁 Total Files Changed

1 file changed.

🧪 Test Added

No explicit tests were added or mentioned in the pull request description. It is highly recommended to add unit tests to cover scenarios where serverResponse.data is nil or does not contain an "error" key, to ensure the new null check behaves as expected and prevents crashes.

🔒 Security Vulnerabilities

No new security vulnerabilities were introduced. This change acts as a defensive programming measure, improving the stability and reliability of the application by preventing potential runtime crashes, which can indirectly contribute to a more secure and resilient system.

Sequence Diagram

sequenceDiagram
    participant Caller as "Network Request Caller"
    participant BNCServerInterface as "BNCServerInterface"
    participant Service as "External Service"
    participant BranchLogger as "BranchLogger"

    Caller->>+BNCServerInterface: genericHTTPRequest(request, retryNumber)
    Note over BNCServerInterface: Handles network requests

    BNCServerInterface->>Service: HTTP Request (NSURLRequest)
    Service-->>BNCServerInterface: serverResponse (NSHTTPURLResponse, data, error)

    alt Network Request Successful (status 2xx)
        BNCServerInterface->>BNCServerInterface: Process serverResponse

        alt serverResponse.data exists AND serverResponse.data["error"] exists
            BNCServerInterface->>BNCServerInterface: Parse errorJson from serverResponse.data["error"]
            BNCServerInterface->>BNCServerInterface: Create NSError object (underlyingError) from errorJson
        else serverResponse.data is nil OR serverResponse.data["error"] is nil
            BNCServerInterface->>BNCServerInterface: underlyingError remains nil (or default)
        end

        BNCServerInterface->>BranchLogger: logError("Giving up on request with HTTP status code %ld", underlyingError)
        BNCServerInterface-->>-Caller: Callback with result/error
    else Network Request Failed (status non-2xx or network error)
        BNCServerInterface->>BranchLogger: logError("Giving up on request with HTTP status code %ld", underlyingError)
        BNCServerInterface-->>-Caller: Callback with network error
    end

/matter review

@gdeluna-branch After this fix, this function will create a long url if short url creation fails. But react native code will not read this URL because error object will be non-nil.

Confirmed this behavior, we don't serve any long links in Android and I'm pretty sure we don't want long links returned anyway. Dropping the object for now on RN side, we can remove all references to long links in another PR.